fix(openai): preserve reasoning for local OpenAI-compatible models via R1 toggle - #1119
Conversation
…a R1 toggle The openAiR1FormatEnabled toggle (UI + provider-settings) only forced the R1 request format, but getModel() never set info.preserveReasoning. As a result Task.ts (shouldPreserveForApi = info.preserveReasoning === true) stripped reasoning_content from follow-up context for every local OpenAI-compatible reasoning model (llama.cpp, LM Studio, Ollama) — there was no way to feed the chain back. Enable the toggle and getModel() now sets preserveReasoning: true so the reasoning chain is preserved in the next-turn context. Default behaviour unchanged.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📜 Recent review details⏰ Context from checks skipped due to timeout. (4)
🧰 Additional context used📓 Path-based instructions (5)Treat model, provider, MCP, path, command, and tool data as untrusted.⚙️ CodeRabbit configuration file Files:
Require regression coverage at the lowest valid harness with behavior-focused assertions, including relevant negative, error, false/unset, and boundary cases.⚙️ CodeRabbit configuration file Files:
Check strict typing and exhaustive behavior across normal, boundary, error, cancellation, retry, and compatibility paths.⚙️ CodeRabbit configuration file Files:
Verify extension/webview contracts, cancellation and error propagation, VS Code lifecycle correctness, and behavior under retries and partial failure.⚙️ CodeRabbit configuration file Files:
Act as an adversarial second-opinion reviewer.⚙️ CodeRabbit configuration file Files:
🔇 Additional comments (1)
📝 SummarySummary by CodeRabbit
Walkthrough
ChangesOpenAI reasoning configuration
Estimated code review effort: 2 (Simple) | ~10 minutes Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to The R1 toggle now preserves streamed reasoning for follow-up requests while default behavior remains unchanged. The focused implementation and tests are ready to merge. 🚥 Pre-merge checks | ✅ 8✅ Passed checks (8 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
This work is a joint effort by chaos (@ch405canova-sudo) and opencode — the bug was found and the fix developed together on a local llama.cpp stack (August 2026). |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
This PR addresses the same root cause as #1096 (local OpenAI-compatible reasoning is discarded because Note on approach: #1096 proposes a separate |
| // toggle, treat the model as preserving reasoning so the chain is fed back. | ||
| return { | ||
| id, | ||
| info: this.options.openAiR1FormatEnabled ? { ...info, preserveReasoning: true } : info, |
There was a problem hiding this comment.
createMessage() treats a model as R1-format via modelId.includes("deepseek-reasoner") || enabledR1Format (line 90), but this gate only checks the toggle. If someone points openAiModelId at a custom endpoint whose id contains deepseek-reasoner without flipping the toggle, wouldn't createMessage still use R1 conversion while preserveReasoning never gets set here — leaving the original bug open for that case?
| // toggle, treat the model as preserving reasoning so the chain is fed back. | ||
| return { | ||
| id, | ||
| info: this.options.openAiR1FormatEnabled ? { ...info, preserveReasoning: true } : info, |
There was a problem hiding this comment.
This forces preserveReasoning: true whenever the toggle is on, even if openAiCustomModelInfo.preserveReasoning was explicitly set to false. Should an explicit value win here, e.g. info.preserveReasoning ?? true?
Co-authored-by: edelauna <54631123+edelauna@users.noreply.github.com>
|
This PR has been awaiting author changes for 14 days and will be automatically closed in 7 days. Please address the review comments or leave a comment if you need more time. |
Review statusThanks for contributing. This comment tracks the review sequence and the next action. Current step: Awaiting fresh human maintainer or CODEOWNER approval. Automated review is complete for the latest commit but does not replace human approval. Review-state labels are managed by this workflow; do not edit them manually. |
Summary
Local OpenAI-compatible reasoning models (llama.cpp
llama-server, LM Studio, Ollama's OpenAI endpoint, vLLM, etc.) stream areasoning_contentfield, but Zoo Code strips it from the follow-up context for these providers — so the model can never see its own reasoning chain on the next turn.Root cause
openAiR1FormatEnabled(exposed as a UI checkbox viaR1FormatSettingand declared inprovider-settings.ts) only forces the R1 request format inopenai.ts. It never propagates toinfo.preserveReasoning, which is what gates reasoning retention inTask.ts:OpenAiHandler.getModel()builds itsModelInfofromopenAiCustomModelInfo ?? openAiModelInfoSaneDefaults— neither setspreserveReasoning. For the built-in OpenAI-compatible provider the value is therefore alwaysundefined, and reasoning is stripped from messages sent back to the API.Fix
When the user enables the R1 format toggle,
getModel()now setspreserveReasoning: trueon the returned model info:This preserves
reasoning_contentin the assistant history sent to the model on follow-up turns. Default behaviour (toggle off) is unchanged.Tests
Added two
getModelcases tosrc/api/providers/__tests__/openai.spec.ts:preserveReasoningistruewhenopenAiR1FormatEnabledis onpreserveReasoningstaysundefinedby defaultRelated
Fixes #1118